-
Notifications
You must be signed in to change notification settings - Fork 304
Password credential type #6207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Password credential type #6207
Conversation
43b6f4c to
b7dd7ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look and noticed there are some conflicting from the LDAP work that needs to be updated here - mainly comments.
However, I also see a bunch of merge commits in this branch from main - we generally avoid that in LLBs to allow for linear history, if we rebase on main instead of merging we can then do a clean merge into main when we are ready for the llb to merge and preserve the commits for the feature.
If you are working alone on a feature and expect to squash the changes when you merge then merging main into your branch is 👍 - but we do not like squashing llbs because we loss the history of who actually worked on what.
internal/db/schema/migrations/oss/postgres/98/02_username_password_domain_vault.up.sql
Outdated
Show resolved
Hide resolved
0aacb75 to
f433810
Compare
f433810 to
64a2243
Compare
hugoghx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, great work everyone! Just a comment ⏬
64a2243 to
06341c0
Compare
06341c0 to
94830f4
Compare
94830f4 to
c39a505
Compare
laero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's wait for some other approvals before we merge since i worked on this PR too
louisruch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great well done - I am aware it was reviewed in a PR coming into the LLB but I would appreciate someone from Quality double checking the e2e infra changes (maybe @moduli)
Description
This PR is an accumulation of the components for implementing the password credential type. This is a brokered only password credential type.
Sample CLI Usage
boundary scopes create -scope-id globalboundary scopes create -scope-id o_fJtwTPHZdMboundary targets create tcp -name "test-target" -scope-id p_SdrMtBv6Zi -default-port 22 -address 127.0.0.1boundary credential-stores create static -scope-id p_SdrMtBv6Ziexport TESTPASS="test-password"boundary credentials create password -credential-store-id csst_POG2btczXC -password env://TESTPASS -name test-pcredboundary targets add-credential-sources -id ttcp_tucp5A15x1 -brokered-credential-source credp_DlHnSSRBpuboundary connect -target-id ttcp_tucp5A15x1